fix(sendStream): add support for file streaming #624
Conversation
GalacticHypernova
changed the title
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #624 +/- ##
==========================================
- Coverage 77.80% 77.78% -0.03%
==========================================
Files 47 47
Lines 4281 4289 +8
Branches 610 612 +2
==========================================
+ Hits 3331 3336 +5
- Misses 933 936 +3
Partials 17 17 ☔ View full report in Codecov by Sentry. |
src/utils/response.ts
Outdated
| @@ -1,5 +1,5 @@ | |||
| import type { OutgoingMessage } from "node:http"; | |||
| import type { Readable } from "node:stream"; | |||
| import { Readable } from "node:stream"; | |||
There was a problem hiding this comment.
We cannot depend on Node.js imports in h3.
There was a problem hiding this comment.
Hmm, that's gonna make it trickier then, because the current stream can't handle buffers..
src/utils/response.ts
Outdated
| @@ -220,7 +220,9 @@ export function sendStream( | |||
| if (!stream || typeof stream !== "object") { | |||
| throw new Error("[h3] Invalid stream provided."); | |||
| } | |||
|
|
|||
| if (Buffer.isBuffer(stream)) { | |||
| stream = Readable.from([stream]); | |||
There was a problem hiding this comment.
I am wondering since the data source is already in memory as Buffer, would there be any benefits of this over simply using res.end(buff) and let Node.js/OS TCP stack handle streaming?
There was a problem hiding this comment.
Is there a guarantee it would send it as a stream?
There was a problem hiding this comment.
It is a TCP stream always. How to be chunked? it depends on many factors including stack and reverse proxy's behavior and the client receives on the other side. But I guess when we have all the data already in a buffer (memory), passing it to the lower level engine makes the right sense.
There was a problem hiding this comment.
@pi0 like this?
|
Thanks for PR. I don't think we should wrap buffer into another stream but instead send it normally by |
🔗 Linked issue
Closes #623
❓ Type of change
📚 Description
This PR adds support for file streaming when used with unstorage by converting the buffer to a readable.
📝 Checklist